New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: attemp DirectPath by default #467
Conversation
Codecov Report
@@ Coverage Diff @@
## master #467 +/- ##
============================================
+ Coverage 80.65% 80.68% +0.02%
+ Complexity 1115 1114 -1
============================================
Files 105 105
Lines 6941 6936 -5
Branches 369 367 -2
============================================
- Hits 5598 5596 -2
Misses 1144 1144
+ Partials 199 196 -3
Continue to review full report at Codecov.
|
@@ -166,12 +163,6 @@ | |||
private EnhancedBigtableStubSettings(Builder builder) { | |||
super(builder); | |||
|
|||
if (DIRECT_PATH_ENDPOINT.equals(builder.getEndpoint())) { | |||
logger.warning( | |||
"Connecting to Bigtable using DirectPath." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is there anywhere we can keep this logging information to confirm (in tests, for example) that we are connecting using DirectPath?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For tests we inject hooks into netty to ensure that directpath is used, also the integration tests generate verbose grpc logs which would show directpath usage. For prod usage, we dont really have a good way to introspect it as its a grpc level feature
// TODO(weiranf): Remove this once DirectPath goes to public beta | ||
private static boolean isDirectPathEnabled() { | ||
return Boolean.getBoolean("bigtable.attempt-directpath"); | ||
.setAttemptDirectPath(true); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add a comment on why we attemt DirectPath? to give context to future readers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added comment. PTAL at the wording, thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
🤖 I have created a release \*beep\* \*boop\* --- ## [1.17.0](https://www.github.com/googleapis/java-bigtable/compare/v1.16.2...v1.17.0) (2020-10-23) ### Features * attemp DirectPath by default ([#467](https://www.github.com/googleapis/java-bigtable/issues/467)) ([89c622d](https://www.github.com/googleapis/java-bigtable/commit/89c622da6038067892687af3edafae743465eda7)) * backup level IAM ([#450](https://www.github.com/googleapis/java-bigtable/issues/450)) ([f38a8ec](https://www.github.com/googleapis/java-bigtable/commit/f38a8ecdc6164d081ef96f748ea37bd62b29b419)) * Implement toString for Bigtable*Settings ([#488](https://www.github.com/googleapis/java-bigtable/issues/488)) ([4d821f8](https://www.github.com/googleapis/java-bigtable/commit/4d821f85ceb237c8e449243ff8c80fb94e22ad51)) ### Bug Fixes * Make refreshing channel compatible with BigtableDataClientFactory ([#474](https://www.github.com/googleapis/java-bigtable/issues/474)) ([fc74164](https://www.github.com/googleapis/java-bigtable/commit/fc741645536e01fac772136bc8346f73ff95e600)) ### Documentation * fix formatting ([#476](https://www.github.com/googleapis/java-bigtable/issues/476)) ([eb24569](https://www.github.com/googleapis/java-bigtable/commit/eb24569e53f9d2b7fde50748c840c2c11f3f3c80)) ### Dependencies * update dependency com.google.cloud:google-cloud-shared-dependencies to v0.12.1 ([#475](https://www.github.com/googleapis/java-bigtable/issues/475)) ([9e56edf](https://www.github.com/googleapis/java-bigtable/commit/9e56edfa7b0a78f14518a99130a7b319c5873be4)) * update dependency com.google.cloud:google-cloud-shared-dependencies to v0.13.0 ([#484](https://www.github.com/googleapis/java-bigtable/issues/484)) ([aad648f](https://www.github.com/googleapis/java-bigtable/commit/aad648fec16b122092d394350822da742a2d7aa0)) * update dependency com.google.truth:truth to v1.1 ([#483](https://www.github.com/googleapis/java-bigtable/issues/483)) ([cca1e0e](https://www.github.com/googleapis/java-bigtable/commit/cca1e0e16f2ec0cc887d81c1844f5395ce08b6ea)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please).
This reverts commit 89c622d.
🤖 I have created a release \*beep\* \*boop\* --- ### [1.17.2](https://www.github.com/googleapis/java-bigtable/compare/v1.17.1...v1.17.2) (2020-11-10) ### Bug Fixes * readRowSettings use manual readRows settings instead of gapics ([#494](https://www.github.com/googleapis/java-bigtable/issues/494)) ([0ef7c5d](https://www.github.com/googleapis/java-bigtable/commit/0ef7c5d4aacacd2030a52efc148ead457719a927)) ### Reverts * Revert "feat: attemp DirectPath by default (#467)" (#520) ([b33b0fc](https://www.github.com/googleapis/java-bigtable/commit/b33b0fc1b5478934298db317c1168c1e9dc20599)), closes [#467](https://www.github.com/googleapis/java-bigtable/issues/467) [#520](https://www.github.com/googleapis/java-bigtable/issues/520) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please).
🤖 I have created a release \*beep\* \*boop\* --- ### [1.17.2](https://www.github.com/googleapis/java-bigtable/compare/v1.17.1...v1.17.2) (2020-11-10) ### Bug Fixes * readRowSettings use manual readRows settings instead of gapics ([googleapis#494](https://www.github.com/googleapis/java-bigtable/issues/494)) ([0ef7c5d](https://www.github.com/googleapis/java-bigtable/commit/0ef7c5d4aacacd2030a52efc148ead457719a927)) ### Reverts * Revert "feat: attemp DirectPath by default (googleapis#467)" (googleapis#520) ([b33b0fc](https://www.github.com/googleapis/java-bigtable/commit/b33b0fc1b5478934298db317c1168c1e9dc20599)), closes [googleapis#467](https://www.github.com/googleapis/java-bigtable/issues/467) [googleapis#520](https://www.github.com/googleapis/java-bigtable/issues/520) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please).
* feat: attemp DirectPath by default * Add comment for DP attempt
🤖 I have created a release \*beep\* \*boop\* --- ## [1.17.0](https://www.github.com/googleapis/java-bigtable/compare/v1.16.2...v1.17.0) (2020-10-23) ### Features * attemp DirectPath by default ([googleapis#467](https://www.github.com/googleapis/java-bigtable/issues/467)) ([89c622d](https://www.github.com/googleapis/java-bigtable/commit/89c622da6038067892687af3edafae743465eda7)) * backup level IAM ([googleapis#450](https://www.github.com/googleapis/java-bigtable/issues/450)) ([f38a8ec](https://www.github.com/googleapis/java-bigtable/commit/f38a8ecdc6164d081ef96f748ea37bd62b29b419)) * Implement toString for Bigtable*Settings ([googleapis#488](https://www.github.com/googleapis/java-bigtable/issues/488)) ([4d821f8](https://www.github.com/googleapis/java-bigtable/commit/4d821f85ceb237c8e449243ff8c80fb94e22ad51)) ### Bug Fixes * Make refreshing channel compatible with BigtableDataClientFactory ([googleapis#474](https://www.github.com/googleapis/java-bigtable/issues/474)) ([fc74164](https://www.github.com/googleapis/java-bigtable/commit/fc741645536e01fac772136bc8346f73ff95e600)) ### Documentation * fix formatting ([googleapis#476](https://www.github.com/googleapis/java-bigtable/issues/476)) ([eb24569](https://www.github.com/googleapis/java-bigtable/commit/eb24569e53f9d2b7fde50748c840c2c11f3f3c80)) ### Dependencies * update dependency com.google.cloud:google-cloud-shared-dependencies to v0.12.1 ([googleapis#475](https://www.github.com/googleapis/java-bigtable/issues/475)) ([9e56edf](https://www.github.com/googleapis/java-bigtable/commit/9e56edfa7b0a78f14518a99130a7b319c5873be4)) * update dependency com.google.cloud:google-cloud-shared-dependencies to v0.13.0 ([googleapis#484](https://www.github.com/googleapis/java-bigtable/issues/484)) ([aad648f](https://www.github.com/googleapis/java-bigtable/commit/aad648fec16b122092d394350822da742a2d7aa0)) * update dependency com.google.truth:truth to v1.1 ([googleapis#483](https://www.github.com/googleapis/java-bigtable/issues/483)) ([cca1e0e](https://www.github.com/googleapis/java-bigtable/commit/cca1e0e16f2ec0cc887d81c1844f5395ce08b6ea)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please).
🤖 I have created a release \*beep\* \*boop\* --- ### [1.17.2](https://www.github.com/googleapis/java-bigtable/compare/v1.17.1...v1.17.2) (2020-11-10) ### Bug Fixes * readRowSettings use manual readRows settings instead of gapics ([googleapis#494](https://www.github.com/googleapis/java-bigtable/issues/494)) ([0ef7c5d](https://www.github.com/googleapis/java-bigtable/commit/0ef7c5d4aacacd2030a52efc148ead457719a927)) ### Reverts * Revert "feat: attemp DirectPath by default (googleapis#467)" (googleapis#520) ([b33b0fc](https://www.github.com/googleapis/java-bigtable/commit/b33b0fc1b5478934298db317c1168c1e9dc20599)), closes [googleapis#467](https://www.github.com/googleapis/java-bigtable/issues/467) [googleapis#520](https://www.github.com/googleapis/java-bigtable/issues/520) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please).
Update client to attempt DirectPath by default, and remove DirectPath test endpoint.
Note that it doesn't mean that after this change client will just use DirectPath, but will call the DirectPath codepath by default.
The actually enablement of DirectPath is controlled by service owner via ACL config. For now, after this change, although all users will attempt DirectPath, but they will all just fallback to the original CFE path.
For testing, we will just use
-Dbigtable.directpath-data-endpoint
and-Dbigtable.directpath-admin-endpoint
to override the default endpoint with DP enabled endpoints.@igorbernstein2
@mgarolera
@mohanli-ml